Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support RGB in HEALpix -> HiPS #141

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Conversation

cdeil
Copy link
Contributor

@cdeil cdeil commented Jan 8, 2019

This PR addresses #136 and adds RGB image support for the HEALPix -> HiPS conversion.

I'll try to add a docs example that shows that it really works before merging this in.

@cdeil cdeil added this to the 0.3 milestone Jan 8, 2019
@cdeil cdeil self-assigned this Jan 8, 2019
@coveralls
Copy link

coveralls commented Jan 8, 2019

Coverage Status

Coverage decreased (-0.3%) to 95.322% when pulling da470d1 on cdeil:healpix-rgb into 3719ae4 on hipspy:master.

@cdeil
Copy link
Contributor Author

cdeil commented Jan 8, 2019

@tboch - Could you please have a look at the docs example here?

Is it possible to show HiPS in Aladin Lite without Allsky.fits using some options?
Or do I have to generate it to make this work?

For Aladin Desktop I also don't manage to load the HiPS, if I just pass the folder name.

Probably there's still bugs and missing information ... ?

@tboch
Copy link
Member

tboch commented Jan 9, 2019

@cdeil: currently, this can not be displayed in Aladin Lite for 2 reasons : AL currently ony supports JPEG/PNG tiles and also requires the presence of Allsky (work in progress to get rid of the latter limitation).

I did not manage either to visualize the generated HiPS in Aladin Desktop, I'll check with Pierre tomorrow, I'm sure there must be a way.

@cdeil
Copy link
Contributor Author

cdeil commented Jan 9, 2019

I'll change the example to PNG and also generate an AllSky. Then it should work in Aladin Lite.
The JS code to load the files from local disk is OK?

@tboch
Copy link
Member

tboch commented Jan 9, 2019

The JS code to load the files from local disk is OK?

almost. It should be:

aladin.setImageSurvey(
        aladin.createImageSurvey(
            'test123', 'test123', '.', 'galactic', 1, {imgFormat: 'png'}
        )
    );

@cdeil
Copy link
Contributor Author

cdeil commented Jan 10, 2019

AL currently ony supports JPEG/PNG tiles and also requires the presence of Allsky (work in progress to get rid of the latter limitation).

That would be nice to make it work without Allsky. Also, if possible, making it work for any order, not just >=3 would be nice. I'll try to change to order=3 for the test and see if that works.

@cdeil
Copy link
Contributor Author

cdeil commented Jan 10, 2019

I've continued with this a bit in 866748a .

I think it would be better to have the order as parameter from the start and to pass it in to all the functions, instead of inferring it from the HEALPix array size and tile width deep down in healpix_to_hips_tile:
https://github.com/hipspy/hips/pull/141/files#diff-4de2f89531f836afdfd881e73aea8632R78
I'll try that now.

@cdeil
Copy link
Contributor Author

cdeil commented Jan 10, 2019

With 6df49aa we now have for the first time generated some HiPS that Aladin Lite can load:

screenshot 2019-01-10 at 17 47 46

However the tiles clearly aren't filled correctly yet. Could be a bug in many places, e.g. the allsky tile handling or the hipsgen part. @tboch - If you have time to have a look, please do. Any suggestions / code review highly welcome. Otherwise I'll try to come back to this and figure it out / finish it up in the coming days.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants